Conversation
BYK
left a comment
BYK
left a comment
There was a problem hiding this comment.
Shall we add black too in a follow up?
src/hpack/hpack.py
Outdated
|
|
||
|
|
||
| def _to_bytes(string): | ||
| def _to_bytes(string: Union[bytes, str, Any]) -> bytes: |
There was a problem hiding this comment.
Just Any probably covers all but I like you calling out the "expected" types. Maybe add a comment about this to avoid confusion in future readers?
| indexed_name = high_byte & 0x0F | ||
| name_len = 4 | ||
| not_indexable = high_byte & 0x10 | ||
| not_indexable = bool(high_byte & 0x10) |
|
|
||
|
|
||
| def table_entry_size(name, value): | ||
| def table_entry_size(name: Union[bytes, str], value: Union[bytes, str]) -> int: |
There was a problem hiding this comment.
Maybe we can abstract this Union[bytes, str] and tuple[bytes, bytes] into type definitions as they are used in a bunch of places. Something like RawHeaderPiece and UntypedHeaderTuple respectively?
There was a problem hiding this comment.
Yes! It was on my ToDo list, but didn't make it for this first PR - happy to be refactored in a future PR!
I'm working on a similar PR for the h2 library as well, so we might want to hold off on type aliases until we have a full picture.
Kriechi
force-pushed
the
typing
branch
2 times, most recently
from
04b0c41 to
1534dfe
Compare
Kriechi
force-pushed
the
typing
branch
2 times, most recently
from
4d840ab to
55e1e7e
Compare
Kriechi
force-pushed
the
typing
branch
2 times, most recently
from
fc455ed to
1702e21
Compare
2 participants
also closes #274